Skip to content

Created some tests for contains and overlap operations to check DE-9IM compliance #44

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Aug 8, 2023
Merged

Created some tests for contains and overlap operations to check DE-9IM compliance #44

merged 1 commit into from
Aug 8, 2023
+1,860 −2

Conversation

3Davydov
Copy link
Contributor

@3Davydov 3Davydov commented Aug 4, 2023

No description provided.

@esabol
Copy link
Contributor

esabol commented Aug 4, 2023

Thank you for this, @3Davydov ! I love more tests.

There's already a "contains_ops" and a "contains_ops_compat" files for testing. I wonder if it would be appropriate to put your "contains" tests in one of those files?

@3Davydov
Copy link
Contributor Author

3Davydov commented Aug 4, 2023

Hello, @esabol !
I think it's still worth making a separate file for this. The tests in the "contains_ops" and "contains_ops_compat" files are relevant only for the current logic.
If it is decided to switch to the DE-9IM model, then it will be necessary first of all to focus on the tests from "contains", and it may be more convenient to work with a separate file.

vitcpp
vitcpp requested changes Aug 5, 2023
View reviewed changes
Copy link
Contributor

@vitcpp vitcpp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@esabol I propose to accept this PR with some trivial changes. Tests do not affect functionality but can be used to learn semantics of operations. I have some ideas how to improve and reorganise it. @3Davydov did a nice analysis of existing algorithms and found that they are not fully compliant with the semantics of operations on figures as described in the DE-9IM model (and OSG Simple Feature Specification). I understand that we can not just change the behaviour of operations to make it compliant but we can update the docs with some notes about differences with reference models.

@3Davydov 3Davydov force-pushed the contains_and_overlaps_test branch from a8202a5 to 23ff77e Compare August 8, 2023 00:17
@vitcpp vitcpp merged commit 8a75bc2 into postgrespro:master Aug 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vitcpp vitcpp vitcpp approved these changes

@esabol esabol esabol approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@3Davydov @esabol @vitcpp